Skip to content

WIP: switch from IJ1 plugins to IJ2 plugins - #133

Open
nanthony21 wants to merge 1 commit into
saalfeldlab:masterfrom
nanthony21:ij2Plugins
Open

WIP: switch from IJ1 plugins to IJ2 plugins#133
nanthony21 wants to merge 1 commit into
saalfeldlab:masterfrom
nanthony21:ij2Plugins

Conversation

@nanthony21

@nanthony21 nanthony21 commented Oct 29, 2021

Copy link
Copy Markdown

Just wanted to open this PR before getting too far along to make sure that this is a change that might be accepted.

The 3 remaining plugins that use the IJ1 style of implementing ij.plugin.Plugin have been replaced with the IJ2 style of using the scijava Plugin annotation and Command interface.

The plugin can now be used from the imagej API just like any other IJ2 plugin. ImageJ.plugin().getPlugin("bdv.ij.BigWarpImagePlusPlugIn") can retrieve a reference to the class and

A macro like is fine:
run("Big Warp")
The main downside is that this no longer seems to work for reasons I don't fully understand:
run("Big Warp", "moving_image=my-moving-img-title.tif target_image=my-target-image-title.tif");

I believe it is a problem with the GenericDialogPlus and will result an an error like this:

(Fiji Is Just) ImageJ 2.3.0/1.53f51; Java 11.0.9.1 [64-bit]; Windows 10 10.0; 49MB of 8178MB (<1%)
 
java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.lang.RuntimeException: Module threw exception
	at net.imagej.legacy.LegacyService.runLegacyCompatibleCommand(LegacyService.java:307)
	at net.imagej.legacy.DefaultLegacyHooks.interceptRunPlugIn(DefaultLegacyHooks.java:166)
	at ij.IJ.runPlugIn(IJ.java)
	at ij.Executer.runCommand(Executer.java:150)
	at ij.Executer.run(Executer.java:68)
	at ij.IJ.run(IJ.java:323)
	at ij.IJ.run(IJ.java:334)
	at ij.macro.Functions.doRun(Functions.java:691)
	at ij.macro.Functions.doFunction(Functions.java:98)
	at ij.macro.Interpreter.doStatement(Interpreter.java:278)
	at ij.macro.Interpreter.doStatements(Interpreter.java:264)
	at ij.macro.Interpreter.run(Interpreter.java:160)
	at ij.macro.Interpreter.run(Interpreter.java:93)
	at ij.macro.Interpreter.run(Interpreter.java:104)
	at ij.plugin.Macro_Runner.runMacro(Macro_Runner.java:162)
	at ij.IJ.runMacro(IJ.java:159)
	at ij.IJ.runMacro(IJ.java:148)
	at net.imagej.legacy.IJ1Helper$3.call(IJ1Helper.java:1148)
	at net.imagej.legacy.IJ1Helper$3.call(IJ1Helper.java:1144)
	at net.imagej.legacy.IJ1Helper.runMacroFriendly(IJ1Helper.java:1095)
	at net.imagej.legacy.IJ1Helper.runMacro(IJ1Helper.java:1144)
	at net.imagej.legacy.plugin.IJ1MacroEngine.eval(IJ1MacroEngine.java:145)
	at org.scijava.script.ScriptModule.run(ScriptModule.java:157)
	at org.scijava.module.ModuleRunner.run(ModuleRunner.java:163)
	at org.scijava.module.ModuleRunner.call(ModuleRunner.java:124)
	at org.scijava.module.ModuleRunner.call(ModuleRunner.java:63)
	at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:225)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.util.concurrent.ExecutionException: java.lang.RuntimeException: Module threw exception
	at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
	at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
	at net.imagej.legacy.LegacyService.runLegacyCompatibleCommand(LegacyService.java:303)
	... 30 more
Caused by: java.lang.RuntimeException: Module threw exception
	at org.scijava.module.ModuleRunner.call(ModuleRunner.java:127)
	... 6 more
Caused by: java.lang.RuntimeException: Macro canceled
	at ij.macro.Interpreter.error(Interpreter.java:1359)
	at ij.macro.Interpreter.abort(Interpreter.java:2099)
	at ij.IJ.error(IJ.java:770)
	at ij.gui.GenericDialog.getNextChoiceIndex(GenericDialog.java:1237)
	at bdv.ij.BigWarpImagePlusPlugIn.run(BigWarpImagePlusPlugIn.java:117)
	at org.scijava.command.CommandModule.run(CommandModule.java:196)
	at org.scijava.module.ModuleRunner.run(ModuleRunner.java:163)
	at org.scijava.module.ModuleRunner.call(ModuleRunner.java:124)
	... 6 more

In order to enable passing arguments to the plugin I think the plugins will have to use the @Parameter annotation as described here

@nanthony21 nanthony21 mentioned this pull request Oct 29, 2021
Open
@bogovicj

bogovicj commented Nov 1, 2021

Copy link
Copy Markdown
Contributor

Thanks @nanthony21,

I appreciate you having a crack at this. It feels pretty evil to break people's macros, so I won't want to merge this
until we're sure that macros behave in the same way. GenericDialogPlus certainly has some idiosyncrasies...
I'll try to have a look at this later this week.

Sounds like this is working for your purposes though, is that right?

@nanthony21

nanthony21 commented Nov 1, 2021

Copy link
Copy Markdown
Author

Yes, this change does allow the plugin to be accessed through ij.plugin().getPlugin("bdv.ij.BigWarpImagePlusPlugIn")

However, I just found out that there is also a different way that I could access the plugin class directly from Python without needing to change how the plugin is loaded to ImageJ:

from scyjava import jimport
MyPlugIn = jimport("my.plugin.MyPlugIn")

So I think I should be able to do what I'm trying to do without getting this merged. Still there may be some benefit to using the updated way of declaring plugins, I'm not sure.

@ctrueden

ctrueden commented Feb 20, 2022

Copy link
Copy Markdown
Collaborator

@bogovicj How to proceed with this PR will depend on your requirements and goals for bigwarp.

Since a requirement is to preserve backwards compatibility, I agree that you cannot just delete the original-ImageJ-style plugins. That said: is it a goal (even if later) to provide a "pure ImgLib2/SciJava" type plugin suite, not depending on net.imagej:ij data model or plugin framework? Unfortunately, this plugin currently has the "worst of all worlds" in that it requires the original ImageJ and ImgLib2 and SciJava Common. One solution could be to split the code in half, migrating all core functionality to a separate bigwarp library, and keeping bigwarp_fiji with the dependency on net.imagej:ij.

I understand that that would be significant work, and you probably don't have time, in which case closing this PR makes sense. Then we can consider this plugin in "maintenance mode" rather than under active development.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nanthony21 @bogovicj @ctrueden